[AL-4886] Introduce ModelSlice #931
Conversation
kozikkamil
requested review from
dubininsergey,
kkim-labelbox,
msokoloff1,
vbrodsky and
whistler
| return PaginatedCollection( | ||
| client=self.client, | ||
| query=query_str, | ||
| params={'id': self.uid}, |
There was a problem hiding this comment.
according to gql above, $first is required, but I do not see it in the params. Is it ok?
There was a problem hiding this comment.
prob handled in PaginatedCollection?
There was a problem hiding this comment.
| query=query_str, | ||
| params={'id': self.uid}, | ||
| dereferencing=['getDataRowIdsBySavedQuery', 'nodes'], | ||
| obj_class=lambda _, data_row_id: data_row_id, |
There was a problem hiding this comment.
not following here... according to PaginatedCollection documentation, obj_class is a class of an object...
There was a problem hiding this comment.
That documentation is 4 years old, I think it's outdated. I was actually just copying the code from CatalogSlice here 😅
|
The change looks good, @kozikkamil can we add a basic fuse test for the method? |
It won't actually be giving any value though. We'll be effectively unit testing PaginatedCollection class, which I am sure has enough coverage already from the e2e tests. It seems like it's not worth to spend the time right now to set that up. ModelSlice has basically no logic in itself |
IMO we should have a fuse test that would catch for example the case where the name of public method has been changed accidentally, or it has been removed at all. If we have something mentioned in SDK docs we should be sure that it's in the code too. Not going to block this though |
Adds the new ModelSlice entity and get method